-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Start resources on demand #118
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is nothing that I have a problem with, so +1 from me.
But I must be missing something because the PR is much more complicated than I would expect.
As "resources on demand" I would expect something like --on-demand
parameter or --tag on-demand
for the resalloc ticket
command. Which would basically just set high priority and ignore all limits to spawn the resource ASAP.
The linked Copr issue says "Provide a few high-performance builders (for demanding software like chromium)" which is IMHO a different concept than on-demand resources. But in this case, I would expect only a new pool configuration that would provide beefy machines.
This PR seems to do more things. Maybe let's discuss on some standup.
else: | ||
assert False | ||
tags.clear() | ||
tags.extend(new_tags) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function deserves some tests. It is very testing-friendly.
ed2f7d2
to
6c37d0c
Compare
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
c3ad2ee
to
49b7a68
Compare
resallocserver/priority_queue.py
Dismissed
@@ -2,6 +2,11 @@ | |||
Python priority queue implementation. | |||
""" | |||
|
|||
# TODO: rename "Task" to "Item" if we want to make this a library, we can | |||
# place any kind of resource into the Queue (Pools, Tickets, etc.). | |||
# TODO: enforce the PriorityQueueTask use. Using the default __repr__ for |
Check warning
Code scanning / vcs-diff-lint
TODO: enforce the PriorityQueueTask use. Using the default repr for Warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer keeping these TODO entries.
952a31d
to
de9be38
Compare
This feature is to get "high performance" builders into Copr: https://github.com/fedora-copr/debate/blob/main/2023-07-28-high-performance-builders.md praiskup/resalloc#118
This feature is to get "high performance" builders into Copr: https://github.com/fedora-copr/debate/blob/main/2023-07-28-high-performance-builders.md praiskup/resalloc#118
This feature is to get "high performance" builders into Copr: https://github.com/fedora-copr/debate/blob/main/2023-07-28-high-performance-builders.md praiskup/resalloc#118
This feature is to get "high performance" builders into Copr: https://github.com/fedora-copr/debate/blob/main/2023-07-28-high-performance-builders.md praiskup/resalloc#118
This feature is to get "high performance" builders into Copr: https://github.com/fedora-copr/debate/blob/main/2023-07-28-high-performance-builders.md praiskup/resalloc#118
This feature is to get "high performance" builders into Copr: https://github.com/fedora-copr/debate/blob/main/2023-07-28-high-performance-builders.md praiskup/resalloc#118
Drop the unused class for now, and add some TODO items. With those many use-cases we have for PriorityQueue, we should have better method naming and typechecking.
On-demand tickets need to be prioritized over the normal tickets (priority queue), otherwise we risk that normal tickets take "on demand" resources. If we want to prefer one "on demand" pool over the other "on demand" (e.g. spot AWS over normal instances), we need to sort them through a priority queue based on tag priority. The fallback for spawn failures (e.g. if SPOT instances don't start) isn't resolved yet, though. Fixes: #24
This feature is to get "high performance" builders into Copr: https://github.com/fedora-copr/debate/blob/main/2023-07-28-high-performance-builders.md praiskup/resalloc#118 Relates: fedora-copr#2834
This feature is to get "high performance" builders into Copr: https://github.com/fedora-copr/debate/blob/main/2023-07-28-high-performance-builders.md praiskup/resalloc#118
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, I started reviewing but forgot to finish it.
But I must be missing something because the PR is much more complicated than I would expect.
We discussed it last week on 1on1 call, so it makes more sense now.
Missing stuff